Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat!: new abigen!, setup_contract_test!, and handling of shared types #763

Merged
merged 106 commits into from
Jan 16, 2023

Conversation

segfault-magnet
Copy link
Contributor

@segfault-magnet segfault-magnet commented Jan 4, 2023

closes: #689
closes: #686
big thanks to @hal3e for his contributions. Love pairing up with the guy :)

As discussed, the only practical way of solving the issue of reusing types between contracts, scripts and predicates, is to generate the bindings at the same time.

Breaking changes (@digorithm )

  • abigen! format changed
  • setup_contract_test! format changed
  • ABIEncoder no longer reexported through fuels_contract.
  • abigen now wraps everything in a wrapper mod called: abigen_bindings
  • Abigen changed its interface.
  • Generated types with non-unique names can now be accessed only by qualifying the type path (i.e. abigen_bindings::some_contract_a_mod::SomeNonUniqueType)

Changes to abigen!

To support the above, abigen! will now be called as follows:

        abigen!(
            Contract(name="ContractA", abi="contract_a-abi.json"),
            Contract(name="ContractB", abi="contract_b-abi.json"),
            Script(name="SomeScript", abi="some_script-abi.json"),
            Predicate(name="SomePredicate", abi="some_predicate-abi.json"),
//            ...
        );

The parsing logic was enhanced to provide compiler hints to the user in case they get it wrong. These hints are tested via trybuild.

Examples:
Invalid program type used:

abigen!(SomeInvalidProgramType(
    name = "SomeName",
    abi = "some-abi.json"
));
error: Unsupported program type. Expected: 'Contract', 'Script' or 'Predicate'
 --> tests/ui/invalid_program_type.rs:3:9
  |
3 | abigen!(SomeInvalidProgramType(
  |         ^^^^^^^^^^^^^^^^^^^^^^

Invalid attributes:

abigen!(Contract(
    name = "SomeName",
    abi = "some-abi.json",
    unknown = "something"
));
error: Unknown attribute! Expected one of: 'name', 'abi'.
 --> tests/ui/unrecognized_attribute.rs:6:5
  |
6 |     unknown = "something"
  |     ^^^^^^^^^^^^^^^^^^^^^

others include:

  • invalid attribute value
  • missing name or abi
  • duplicate attributes, showing the original one and each duplicate
  • ...

If there are identical types, both in name and in their innards, they will be extracted to a separate module called shared_types. It looks something like this:

mod abigen_bindings{
mod contract_a_mod {
// types and bindings from ContractA here
}
mod contract_b_mod {
// ...
}
// ...
mod shared_types {
// all shared types here
}
}

Handling shared types

If any type or function inside contract_a_mod, contract_b_mod... uses a shared type, it will reference it via super::shared_types::SomeSharedType. It has to use relative references, like super since the macro doesn't know in which mod it is being expanded into.

In addition, due to the symbol collisions in #686, use statements are not injected into the generated code. Everything is referred to by its full path. a Vec is ::std::vec::Vec, String is ::std::string::String, etc.

The std prelude is disabled for the generated code and a smaller, custom one, is used in its place with mostly traits inside, which can't collide with bindings ATM.

Also since we now have the possibility of symbol collision, we cannot do pub use contract_x_mod::* like we did before.
Instead, Abigen will figure out a set of uniquely named symbols and only generate pub use statements for that set. Types that didn't get pub use statements will need to be referred to by their exact path by the user.

There is one caveat to this:
Suppose we have ContractA with type SomeType and ContractB with type SomeType. Also, suppose the two SomeTypes have different internals, so they are not deemed to be 'shared types'.

Currently, we will not generate pub use statements due to the fact that we're going to cause a collision.

In the previous versions, if you ran abigen! twice, once for each contract. You'd get the following:

mod contract_a_mod {
  struct SomeType { //.... }
}
pub use contract_a_mod::*;

mod contract_b_mod {
 struct SomeType { //... }
}
pub use contract_b_mod::*;

this would work even with the collision due to how ::* works -- it will disregard the contract_a_mod::SomeType due to it being 'overriden' by the second abigen!.

I'm not sure if this behavior is useful for anybody, it actually took me by surprise.

But it is worth a mention since calling abigen! twice is still a thing, but will now produce a compile error since you'd end up with:

pub use contract_a_mod::SomeType;

// later
pub use contract_b_mod::SomeType;

Which is a collision, it doesn't have the ::* overriding properties. The two pub use statements are generated because the two invocations of abigen! didn't know about each other.

I don't see this as much of a problem since you can always wrap the abigen!s in a mod, or, do the better thing, generate everything inside one abigen! so it knows not to put the pub use statement for the non-unique symbol at all.

Parent wrapping module

The result of abigen! is now wrapped inside the mod abigen_mod to avoid an edgecase regarding tests and shared types.

Refactoring

Abigen went under a bit of refactoring to make having multiple contracts, scripts and predicates easier.

Generating types, wrapping in mods and other common operations have been lifted to the Abigen, while contract, script, or predicate-specific code resides in their respective files.

You'll also come across other helpers like GeneratedCode for merging and keeping track of the generated types so that we can generate use statements later on.

FunctionGenerator tries to extract as much of the duplication in the various expand_fn functions as it can.

Also, the structure has been shifted around a bit in the code_gen, choices were guided by a dependency graph striving to minimize inter-file dependencies.

setup_contract_test macro rehaul

Following the example of abigen! @hal3e and I have implemented something similar for setup_contract_test:
Here is an example of what the new usage looks like:

    setup_contract_test!(
        Wallets("wallet"),
        Abigen(
            name = "RequireContract",
            abi = "packages/fuels/tests/contracts/require"
        ),
        Deploy(
            name = "contract_instance",
            contract = "RequireContract",
            wallet = "wallet"
        ),
        Deploy(
            name = "contract_instance2",
            contract = "RequireContract",
            wallet = "wallet"
        ),
    );

In short, Wallet creates wallets and launches a provider with them, Abigen does the abigen and Deploy does the deploying :D

We've put some effort into cool compile-time error messages, i.e. trying to Deploy a contract you don't have, and similar checks.

Copy link
Contributor

@hal3e hal3e left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome work! 🎉 I really like it!

packages/fuels-core/src/code_gen/generated_code.rs Outdated Show resolved Hide resolved
packages/fuels-core/src/code_gen/resolved_type.rs Outdated Show resolved Hide resolved
packages/fuels-core/src/code_gen/utils.rs Outdated Show resolved Hide resolved
segfault-magnet and others added 2 commits January 16, 2023 14:56
iqdecay
iqdecay previously approved these changes Jan 16, 2023
Copy link
Contributor

@iqdecay iqdecay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AWESOME WORK. I left a lot of comments because you know I love to nitpick, but I'm super impressed by how clean this all is, and I don't think they are actually "requested changes". So I'm putting it on "approve" because we all want this to be merged fast haha.

docs/src/contracts/the-abigen-macro.md Outdated Show resolved Hide resolved
docs/src/contracts/the-abigen-macro.md Outdated Show resolved Hide resolved
docs/src/contracts/the-abigen-macro.md Outdated Show resolved Hide resolved
docs/src/contracts/the-abigen-macro.md Outdated Show resolved Hide resolved
docs/src/contracts/the-abigen-macro.md Outdated Show resolved Hide resolved
packages/fuels-core/src/code_gen/custom_types/utils.rs Outdated Show resolved Hide resolved
packages/fuels/tests/bindings.rs Outdated Show resolved Hide resolved
hal3e
hal3e previously approved these changes Jan 16, 2023
Copy link
Contributor

@hal3e hal3e left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Congrats again on the amazing work you did!

@segfault-magnet segfault-magnet dismissed stale reviews from hal3e and iqdecay via 7055956 January 16, 2023 15:17
@segfault-magnet segfault-magnet enabled auto-merge (squash) January 16, 2023 19:35
Copy link
Member

@digorithm digorithm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Phew... what a journey. Excellent work; I love the refactoring you did along the way.

I did the best I could, but I'm sure I could miss something; that's why long PRs are discouraged.

This is amazing, let's not do it again 😂

@segfault-magnet segfault-magnet merged commit a8c4586 into master Jan 16, 2023
@segfault-magnet segfault-magnet deleted the segfault-magnet/dedupe_types branch January 16, 2023 21:53
@digorithm
Copy link
Member

@segfault-magnet @iqdecay I've resolved some of the unresolved comments because they were all nits/discussions (plus, it was under approval anyway). I did this so we could merge this in a timely fashion. Feel free to continue these discussions in separate issues and tackle those in separate PRs if applicable!

@iqdecay
Copy link
Contributor

iqdecay commented Jan 17, 2023

@segfault-magnet apart from the subject of the setup_contract_test I don't think anything warrants an issue? Maybe renaming the getters?

@segfault-magnet
Copy link
Contributor Author

Yep, we could do the getters, will write a task along with the setup_contract_test one (which will probably be a discussion if github has such a button :) )

@iqdecay
Copy link
Contributor

iqdecay commented Jan 17, 2023

Created the getters issue (and will PR it soon), I'll let you create the discussion and tag me 😸

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
abigen breaking Introduces or requires breaking changes enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Handle shared types between different contracts Abigen finds std::string::String over sway-lib's String
5 participants